Skip to content

[SPM] Support Swift Package Manager#29

Open
notbenoit wants to merge 1 commit intorefactor/combinefrom
refactor/combine-spm
Open

[SPM] Support Swift Package Manager#29
notbenoit wants to merge 1 commit intorefactor/combinefrom
refactor/combine-spm

Conversation

@notbenoit
Copy link
Copy Markdown

Feat

  • Added SPM support to the Combine branch.

Note

As SPM still lacks support for testing other than for the macOS platform, I decided not to include a test target.

@notbenoit notbenoit force-pushed the refactor/combine-spm branch from cecc52b to 7e0b943 Compare February 18, 2022 15:56
Copy link
Copy Markdown

@hadiidbouk hadiidbouk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was checking something that fails in the immi project and I saw this PR 👍🏻

Comment thread Package.swift
linkerSettings: [
.linkedFramework("Foundation"),
.linkedFramework("UIKit", BuildSettingCondition.when(platforms: [.iOS, .tvOS])),
.linkedFramework("AppKit", BuildSettingCondition.when(platforms: [.macOS]))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can omit the BuildSettingCondition from here 🤔

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I don't even think the linkerSettings section is needed though. I'll try removing the section and see how that goes.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think It's similar to using UIKit that already contains Foundation?
By importing the DataSource we can then remove the UIKit or Foundation imports from our code?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hadiidbouk nope, the import are not transitive, we still need to import Foundation & import UIKit when using the relevant APIs. When using an ObjC bridging header however, Foundation is usually implicitly imported

Comment thread Package.swift
.target(
name: "DataSource",
path: "DataSource",
exclude: ["Info.plist"],
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are excluding the Info.plist file to avoid conflicts when adding this library to a project?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to exclude it otherwise the build system tries to compile the plist file, and fails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants